「關於函式的首要準則,就是要簡短。第二項準則,就是要比第一項的簡短函式還要更簡短。這是一個我無法證明的主張」
「我曾經寫過令人難受的 3000 行函式怪物,寫過數不清的 100 至 300 行大小的函式,也寫過只有 20 到 30 行的函式。這些經驗告訴我,函式應該要非常簡短」
取自: Clean Code (p.40)
先來個例子,請試著瀏覽下列 Code [1]並大致想像功能:
public class HtmlUnit {
public static String testableHtml(
PageData pageData,
boolean includeSuiteSetup
) throws Exception {
WikiPage wikiPage = pageData.getWikiPage();
StringBuffer buffer = new StringBuffer();
if (pageData.hasAttribute("Test")) {
if (includeSuiteSetup) {
WikiPage suiteSetup =
PageCrawlerImpl.getInheritedPage(
SuiteResponder.SUITE_SETUP_NAME, wikiPage
);
if (suiteSetup != null) {
WikiPagePath pagePath =
suiteSetup.getPageCrawler().getFullPath(suiteSetup);
String pagePathName = PathParser.render(pagePath);
buffer.append("!include -setup .")
.append(pagePathName)
.append("\n");
}
}
WikiPage setup =
PageCrawlerImpl.getInheritedPage("SetUp", wikiPage);
if (setup != null) {
WikiPagePath setupPath =
wikiPage.getPageCrawler().getFullPath(setup);
String setupPathName = PathParser.render(setupPath);
buffer.append("!include -setup .")
.append(setupPathName)
.append("\n");
}
}
buffer.append(pageData.getContent());
if (pageData.hasAttribute("Test")) {
WikiPage teardown =
PageCrawlerImpl.getInheritedPage("TearDown", wikiPage);
if (teardown != null) {
WikiPagePath tearDownPath =
wikiPage.getPageCrawler().getFullPath(teardown);
String tearDownPathName = PathParser.render(tearDownPath);
buffer.append("\n")
.append("!include -teardown .")
.append(tearDownPathName)
.append("\n");
}
if (includeSuiteSetup) {
WikiPage suiteTeardown =
PageCrawlerImpl.getInheritedPage(
SuiteResponder.SUITE_TEARDOWN_NAME,
wikiPage
);
if (suiteTeardown != null) {
WikiPagePath pagePath =
suiteTeardown.getPageCrawler().getFullPath (suiteTeardown);
String pagePathName = PathParser.render(pagePath);
buffer.append("!include -teardown .")
.append(pagePathName)
.append("\n");
}
}
}
pageData.setContent(buffer.toString());
return pageData.getHtml();
}
}
P.S. 筆者先自首,這是我第三次閱讀本書,事實上我沒有一次花超過 10 秒鐘在看這段 Code... 匆匆瀏覽的感想是這應該是一段跟 Html Render 有關的 Code,帶有 Mock (Test) 的功能切換、也許還做了一些不明的 Setup?
上述的 Code 不僅符合前面所提到的命名、就連縮排風格筆者也用 Formatter 美化過了 (原書中更亂)
究竟出了什麼問題,導致程式碼的可讀性下降?
接下來我們透過提取幾個函式來重構上面的 Code...
public class HtmlUnit {
public static String renderPageWithSetupsAndTeardowns(
PageData pageData,
boolean isSuite
)
throws Exception {
boolean isTestPage = pageData.hasAttribute("Test");
if (isTestPage) {
WikiPage testPage = pageData.getWikiPage();
StringBuffer newPageContent = new StringBuffer();
includeSetupPages(testPage, newPageContent, isSuite);
newPageContent.append(pageData.getContent());
includeTeardownPages(testPage, newPageContent, isSuite);
pageData.setContent(newPageContent.toString());
}
return pageData.getHtml();
}
}
我想上述的 Code 已經 Clean 到不需要註解和文字介紹了,任何修過程式設計的學生應當都能猜出這段 Code 在做什麼了。順帶一提,上面的函式是可測試的 (Testable),我們會在後面的章節介紹「測試驅動設計 (TDD)」
public static String renderPageWithSetupsAndTeardowns(
PageData pageData, boolean isSuite) throws Exception {
if (isTestPage(pageData))
includeSetupAndTeardownPages(pageData, isSuite);
return pageData.getHtml();
}
甚至只有 2, 3 或 4 行 (關於這點筆者是持保留看法的...)「函式應該只做一件事情」
思考:何謂「一件事」?
上述例子其實做了三件事:
那麼該如何判斷呢?
「函式只做函式名稱下 『同一層抽象概念』 的幾個步驟」
因此,判斷函式是否做超過「一件事」的方法為
「看你是否能從此函式中,提煉出另一個新函式」
且,此新函式的提取會導致抽象概念的進一步簡化或改變
class Bird {
double getSpeed() {
switch (type) {
case EUROPEAN:
return getBaseSpeed();
case AFRICAN:
return getBaseSpeed() - getLoadFactor() * numberOfCoconuts;
case NORWEGIAN_BLUE:
return (isNailed) ? 0 : getBaseSpeed(voltage);
}
throw new RuntimeException("Should be unreachable");
}
}
上述的 Switch 內包含了太多細節了。這導致此函式破壞了 「單一職責原則 (SRP)」 及 「開放封閉原則 (OCP)」 (筆者會在 Clean Architecture 篇詳細介紹此類設計原則) abstract class Bird {
abstract double getSpeed();
}
class European extends Bird {
double getSpeed() {
return getBaseSpeed();
}
}
class African extends Bird {
double getSpeed() {
return getBaseSpeed() - getLoadFactor() * numberOfCoconuts;
}
}
class NorwegianBlue extends Bird {
double getSpeed() {
return (isNailed) ? 0 : getBaseSpeed(voltage);
}
}
speed = bird.getSpeed();
透過這樣子的更改,不僅封裝了底層細節、也提升了程式的可擴充性和維護性。詳細的說明讀者可參見 Reference參數數量,最理想的是 0 個,至多用到 3 個
無論如何都不該超過 3 個參數,除非有非常特殊的理由
includeSetupPage()
比 includeSetupPage(newPageContent)
更容易理解。因為參數會強迫你去瞭解更多目前不重要的細節
Circle makeCircle(double x, double y, double radius);
// 將相似概念的參數放在一起
Circle makeCircle(Point center, double radius);
避免輸出型參數 (Output Parameter)
StringBuffer transform(StringBuffer in)
會比 void transform(StringBuffer out)
更洽當不要使用旗標參數 (Flag Parameter)
「使用旗標參數是一種非常爛的做法」
與其將 boolean 值傳遞給函式,不如直接 Return 處理完後的 boolean 值。或者直接拆成不同函式
render(true)
render(false)
// vs.
renderForSuite()
renderForSingleTest()
「回傳 null 是在給自己增加額外的工作量,也是在給呼叫者找麻煩」
「傳遞 null 到方法裡是更糟糕的行為,應該盡可能避免傳遞 null」
取自: Clean Code (pp.123-124)
「函式應該要能做某件事,或能回答某個問題,但兩者不該同時發生」
例子:
// Confusing
if (set("name", "bob")){
...
}
vs.
// Concrete
if (attributeExists("name")){
setAttribute("name", "bob");
}
[補充]: Command 和 Query 的混雜不僅在代碼層級會造成閱讀混淆,考慮到 Database 大量讀寫的情境,則可能導致一致性 (Consistency) 和權限控管不易的問題
上升到架構層面後衍生出 「命令與查詢分離 (CQS)」 、 「命令與查詢責任隔離 (CQRS)」 ...等模式
可參見 Reference [10], [11]
public void delete(Page page) {
try{
deletePageAndAllReferences(page);
}
catch (Exception e){
logError(e);
}
}
private void deletePageAndAllReferences(Page page) throws Exception{
// ...
}
「雖然 Clean Code 是易讀的,但它也必須是耐用的。當我們將錯誤處理看作是另一件重要的事,將之處理成獨立於主要邏輯的可讀程式,代表我們寫出了整潔又耐用的程式碼。在程式的維護性方面也向前邁進了一大步」
取自: Clean Code (p.126)
以四則運算為例子 [13],有時候我們只關心運算過程如何出錯,例如 Catch 到 "ArithmeticException",對於更詳細的 "DivideByZeroException" 則非呼叫者 (四則運算器) 所關注的細節
可以透過 Wrapper 設計技巧讓程式只回傳共用的例外型態
LocalPort port = new LocalPort(0);
try
{
port.open();
}
catch (PortDeviceFailure e)
{
// error logging...
}
finally
{
// ...
}
public class LocalPort
{
private ACMEPort innerPort;
public LocalPort(int portNumber)
{
innerPort = new ACMEPort(portNumber);
}
public void open() {
try
{
innerPort.open();
}
catch (DeviceResponseException e)
{
throw new PortDeviceFailure(e);
}
catch (ATM1212UnlockedException e)
{
throw new PortDeviceFailure(e);
}
catch (GMXError e)
{
throw new PortDeviceFailure(e);
}
}
}
上述包裹第三方函式庫的做法是非常好的技巧,可以減少對第三方 API 的依賴